Add /health/ready application readiness check - #781
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Reword shipped source comments to be self-contained instead of pointing at an untracked local roadmap doc (StartupHealthCheck.cs, ServiceCollectionExtensions.cs, ApplicationBuilderExtensions.cs) - Add ArgumentNullException.ThrowIfNull guard in StartupHealthCheck constructor - Add a comment warning that every registered health check must carry a live or ready tag - StartupHealthCheckTests: reset DataSettingsManager._instance in Cleanup(), replace null-forgiving operator with an explicit null check, add [DoNotParallelize] to the test class - Plan: clarify which verification method covers the 503 case - Spec: document DataSettingsManager.DatabaseIsInstalled() caching caveat and the need for a restart after install
The design spec and implementation plan under docs/superpowers/ were working artifacts of the agentic workflow, not established documentation for this repo (no prior convention here) and not requested output. The one piece of durable rationale they carried - that DataSettingsManager .DatabaseIsInstalled() caches after its first call and can only be forced to false, never back to true, without a process restart - now lives as a comment on StartupHealthCheck instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type: feature
Issue
The application only exposes
/health/live, a health check that always returnsHealthyregardless of application state. There is no/health/ready, so an orchestrator (e.g. Kubernetes) cannot distinguish "process is alive" from "instance has finished starting and is configured to serve traffic" — it may route traffic to an instance that is still starting up or was deployed before the install wizard ran.Per explicit scope decision, this PR does not add MongoDB or Redis connectivity checks to readiness — only the application process itself is checked. Checking those dependencies is a separate, larger change (tracked as a future extension in the local architecture audit, not part of this PR).
Solution
StartupHealthCheck(Grand.Web.Common.Infrastructure.HealthChecks), anIHealthCheckthat isHealthyonly once:IHostApplicationLifetime.ApplicationStartedhas fired (allIStartupApplication.Configurecalls and hosted services have completed startup), andDataSettingsManager.DatabaseIsInstalled()reports a configured connection string (in-memory/local state, no network I/O — no MongoDB/Redis probing anywhere in this change)."self"check as"live"(unchanged behavior) and registeredStartupHealthCheckas"startup"tagged"ready"./health/liveand/health/readywithHealthCheckOptions.Predicatefiltering on those tags, so/health/livekeeps its exact existing behavior and/health/readyreports true application readiness.Design spec and implementation plan are included under
docs/superpowers/for reference.Breaking changes
None.
/health/livebehavior and response are unchanged./health/readyis a new endpoint; nothing previously depended on it.Testing
dotnet test src/Tests/Grand.Web.Common.Tests/Grand.Web.Common.Tests.csproj— 18/18 pass, including the 3 newStartupHealthCheckTests.dotnet run --project src/Web/Grand.Web/Grand.Web.csproj).curl -i http://localhost:5000/health/live→200 OK, bodyHealthy, regardless of install/DB state.curl -i http://localhost:5000/health/ready→200 OK, bodyHealthyonce the app has started and a database connection is configured;503 Service Unavailablebefore that (e.g. on an instance that hasn't run the install wizard yet).Update: dropped the
docs/superpowers/design-spec/plan files from this branch — they were working artifacts of the agentic workflow with no prior convention in this repo, not requested output. The one durable piece of rationale they carried (theDataSettingsManager.DatabaseIsInstalled()one-way caching gotcha) now lives as a comment onStartupHealthCheck.